-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cascade terminate purge #662
Conversation
821bd52
to
51f027f
Compare
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
328a10c
to
8264b41
Compare
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #662 +/- ##
=======================================
Coverage 86.21% 86.21%
=======================================
Files 79 79
Lines 3998 3998
=======================================
Hits 3447 3447
Misses 551 551 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Shivam Kumar <[email protected]>
Signed-off-by: Shivam Kumar <[email protected]>
@@ -88,6 +96,8 @@ jobs: | |||
uses: actions/setup-go@v5 | |||
with: | |||
go-version: ${{ env.GOVER }} | |||
env: | |||
GOPATH: ${{ github.workspace }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it was failing without it with error GoPath
not set
@@ -56,7 +56,15 @@ jobs: | |||
echo "CHECKOUT_REF=${{ github.event.client_payload.pull_head_ref }}" >> $GITHUB_ENV | |||
echo "DAPR_REF=master" >> $GITHUB_ENV | |||
fi | |||
|
|||
- name: Parse workflow dispatch payload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you refactored it to here. That's fine - hope it works :)
@@ -25,7 +25,7 @@ packages = find_namespace: | |||
include_package_data = True | |||
install_requires = | |||
dapr-dev >= 1.11.0rc1.dev | |||
durabletask >= 0.1.0 | |||
durabletask >= 0.1.1a1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You really shouldn't use alpha versions as dependencies in this SDK. Please use a proper versioning scheme on the durable task side :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update once a stable version is released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
If you update the proto please make sure to share the runtime commit / tag from which the proto client was generated. (And ideally which versions of grpc-tools or protoc you used)
Description
Adds support for cascade terminate/purge in dapr workflow.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #661
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: